build: make the tree compile on gcc 16, and add a CI lane so it stays that way - #945
build: make the tree compile on gcc 16, and add a CI lane so it stays that way#945jimmykarily wants to merge 1 commit into
Conversation
…it that way Five files call ::getpid() without including <unistd.h> and compiled only because an older libstdc++ pulled the header in transitively. gcc 16 does not, so they fail. One of them is src/vllm/entrypoints/openai/server_main.cpp, i.e. a SHIPPED binary that does not build on a current distro. This is the second time this exact break has landed. The first three files were fixed after a gfx1151 report on mudler#41; the pattern came back in five more, because every Linux CI lane installs the distro g++ (gcc 13 on ubuntu-latest) and no lane uses anything newer. A point fix does not hold, so this adds the missing coverage rather than only the includes: * build-newest-gcc: a COMPILE-ONLY lane in the gcc:16 container, tests configured ON so the test tree is compiled too. It deliberately does not run ctest -- build-test-cpu already owns execution, and this lane exists to catch compile-time portability for roughly one extra build per PR. * cmake/CompilerWarnings.cmake: on gcc >= 16 only, -Warray-bounds is reported but no longer fatal. gcc 16 emits it inside libstdc++ and vendored nlohmann for correct code, and no change to the calling code avoids it: after inlining, _Sp_counted_base::_M_release() is identical for every shared_ptr type, so GCC attributes one instantiation's destructor to another's allocation size. libstdc++ carries its own #pragma suppressions around that destructor; upstream is GCC PR tree-optimization/122197. gcc <= 15 is unchanged and still fails the build on a real out-of-bounds. * docs/USAGE.md: a Host compilers section stating which compilers build the tree and what the gcc 16 warning policy is. Required by check-doc-checkpoint, which classifies src/vllm/entrypoints/ as a user-facing surface and has no bypass by design. Verified on gcc 16.1.1 (Arch): full CPU configure + build with tests ON, no command-line workaround, 0 errors, and build-gcc16/examples/vllm-server runs (`vllm.cpp 0.0.3 c-abi=20`). check-doc-checkpoint, check-public-doc-tables, check-readme-structure and check-env-doc all pass. The workflow file itself is NOT verified -- there is no way to execute a GitHub Actions lane locally, so this PR's own CI is its first run. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode] Signed-off-by: Dimitris Karakasilis <dimitris@karakasilis.me>
localai-org-maint-bot
left a comment
There was a problem hiding this comment.
The GCC 16 lane is useful, but the current head introduces a Windows portability regression while fixing the Linux one.
All five source changes add <unistd.h> unconditionally. MSVC does not provide that header, and these targets are part of the Windows build/test surface. Current main already fixed server_main.cpp with the portable split (<process.h> under _WIN32, POSIX handling otherwise), so rebasing the branch will also collide with or supersede that part of this diff. The four loader tests still call ::getpid() and need the same portable treatment (or a shared cross-platform process-ID helper), not an unconditional POSIX include.
The branch is currently 18 commits behind main and has no checks reported. Please rebase, preserve only the still-needed GCC 16 fixes, make the process-ID use portable on Windows, and let the new GCC lane plus the Windows lanes run on the updated head. I can re-review that result.
What this fixes
The project does not currently build with gcc 16, which is what current Linux distributions ship. Five files use
getpid()without including the header that declares it. They compiled anyway on older compilers, because some other standard header happened to include it for them. Newer compilers do not, so the build stops:One of those five is
src/vllm/entrypoints/openai/server_main.cpp. That is the OpenAI server's own entry point, so this is not just a test problem. A binary the project ships cannot be built on a current distribution.Why it keeps happening
This is the second time. Three files were fixed a week ago after a report on #41. The same mistake then appeared in five new files, because each new model-loader test copies the same helper.
The reason nobody notices is that CI cannot see it. There are eighteen CI jobs, including Windows and ARM, but every Linux job installs the distribution's default
g++, which onubuntu-latestis gcc 13. No lane uses anything newer. So the mistake is invisible to CI and to anyone developing on Ubuntu, and it only shows up for people on newer distributions.Fixing the five files again would not stop a third time. So this adds the missing coverage instead.
What is in this PR
1. The five missing includes. One line each.
2. A new CI job,
build-newest-gcc. It builds the project in the officialgcc:16container. It only compiles, it does not run the tests, because the existingbuild-test-cpujob already runs them and this one exists to catch compile problems. Cost is roughly one extra build per pull request. It follows the same pattern as the existing CUDA job, which also runs in a container.3. One warning is downgraded, on gcc 16 and newer only. Without this the new job cannot pass. gcc 16 reports an
array-boundsproblem inside the standard library and the bundled JSON library, on code that is correct. The warning still appears, it just no longer stops the build. On gcc 15 and older nothing changes at all, and a genuine out-of-bounds still fails the build there.Worth knowing about item 3: this is not something the code can be rewritten to avoid. The warning appears after the compiler merges the shared-pointer cleanup function, which is identical for every shared pointer type, and then blames one type's destructor for another type's size. The standard library itself silences this same warning in the same place. The upstream bug is
tree-optimization/122197, and Eigen, assimp and CMSSW all handle it the same way.4. A
Host compilerssection indocs/USAGE.md. Not originally planned.check-doc-checkpointclassifiessrc/vllm/entrypoints/as a user-facing surface and has no bypass by design, so rather than write a filler documentation update, this states the truthful user-facing fact: which compilers build the tree, and what the gcc 16 warning policy is.What I checked, and what I did not
Checked on gcc 16.1.1 (Arch Linux):
vllm.cpp 0.0.3 c-abi=20.check-doc-checkpoint,check-public-doc-tables,check-readme-structureandcheck-env-docall pass.Not checked: the CI file itself. There is no way to run a GitHub Actions job locally, so this pull request's own CI run will be the first time that file executes. If it needs adjusting, say so and I will fix it.
If you would prefer something different
Item 3 is a policy decision about the
-Werrorrule, so it is yours to make. Alternatives, and I am happy to switch to any of them:-Werroruntouched and mark the new job as non-blocking, so it reports the problem without failing the build.Note
This change was prepared with the help of Claude AI and reviewed by the contributor before submission.